Skip to content

Conversation

@devmadhuu
Copy link
Contributor

What changes were proposed in this pull request?

When a cache metrics being registered for any OM DB table, it should check if cache metrics already registered or not. If it is already registered, then we should unregister first and allow registration.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8734

How was this patch tested?

This patch was tested using existing unit test cases including initialisation of new snapshot OM DB in Recon and do registration of cache metrics tables.

@devmadhuu
Copy link
Contributor Author

@sumitagrawl @ChenSammi - pls review

@devmadhuu devmadhuu changed the title HDDS-8734. Make adding cache metrics for OM DB tables an Idempotent operation. HDDS-8734. Make registering cache metrics for OM DB tables an Idempotent operation. May 31, 2023
Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devmadhuu thanks working over this, plz add some test case for this

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @devmadhuu for working on this.

@devmadhuu
Copy link
Contributor Author

@DaveTeng0 - pls review for acceptance test impact

@adoroszlai adoroszlai dismissed their stale review May 31, 2023 16:37

updated

@adoroszlai
Copy link
Contributor

Thanks @devmadhuu for updating the patch.

@adoroszlai
Copy link
Contributor

@devmadhuu acceptance test failure is unrelated, already encountered the same on master (HDDS-8732)

@adoroszlai adoroszlai changed the title HDDS-8734. Make registering cache metrics for OM DB tables an Idempotent operation. HDDS-8734. OM DB cache metrics creation should be idempotent May 31, 2023
@devmadhuu
Copy link
Contributor Author

@devmadhuu thanks working over this, plz add some test case for this

Thanks @sumitagrawl for reviewing. I have added the test case. Pls re-review.

@devmadhuu
Copy link
Contributor Author

@devmadhuu acceptance test failure is unrelated, already encountered the same on master (HDDS-8732)

Thank you @adoroszlai for review.

@adoroszlai adoroszlai requested a review from sumitagrawl June 1, 2023 05:12
Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1

@devmadhuu
Copy link
Contributor Author

@adoroszlai - Pls re-review and if all looks good, pls merge.

@adoroszlai adoroszlai merged commit c374087 into apache:master Jun 1, 2023
@swamirishi
Copy link
Contributor

@adoroszlai As part of #4678 , I have fixed the test case can you review this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants